Skip to content

Jmx unit semconv alignment - Tomcat #13650

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 31 commits into from
May 29, 2025

Conversation

robsunday
Copy link
Contributor

@robsunday robsunday commented Apr 4, 2025

Changes:

  1. Metric names made semconv aligned
  2. Attribute names made semconv aligned
  3. Metrics descriptions format standardized
  4. tomcat.yaml moved from agent to library folder to make it reusable in all library dependants
  5. Tomcat JMX metrics Integration test implemented. It utilize assertions and target system integration test framework ported by @SylvainJuge from opentelemetry-java-contrib

Tomcat metrics description md file moved to library
Code review followup
@robsunday robsunday marked this pull request as ready for review April 8, 2025 13:10
@robsunday robsunday requested a review from a team as a code owner April 8, 2025 13:10
@SylvainJuge
Copy link
Contributor

@robsunday #13597 has been merged, so you can update with current state of main to make diff simpler.

@robsunday robsunday requested review from trask and SylvainJuge May 23, 2025 13:54
Copy link
Contributor

@SylvainJuge SylvainJuge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that even if the stable dotnet runtime metrics for thread pools currently have a different approach, we should attempt to prefer the dotted namespace naming strategy, both for the thread pools and the tomcat request processor (but I agree that it might be a matter of taste here).

While the tomcat "request processor", I still think it belongs to the tomcat.request.* namespace, so I would lean towards using tomcat.request.processor.name instead, also this would make the tomcat.request.* metrics share a common namespace prefix as the their metric attributes. Also, we already have a similar pattern with the jvm.memory.used metric where the attributes are jvm.memory.type and jvm.memory.pool.name.

@robsunday
Copy link
Contributor Author

So, let's keep consistency with jvm. I'll push the update next week

@SylvainJuge
Copy link
Contributor

Thanks a lot for your patience and efforts @robsunday !!

@robsunday
Copy link
Contributor Author

No problem! @SylvainJuge - thanks for valuable comments!

@robsunday robsunday requested a review from trask May 29, 2025 12:30
@trask trask merged commit 4ec198f into open-telemetry:main May 29, 2025
88 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants